Skip to content

feat: add correctness traversal and verification-sufficiency passes - #83

Merged
shaug merged 1 commit into
mainfrom
scott/53-correctness-traversal-verification
Jul 29, 2026
Merged

feat: add correctness traversal and verification-sufficiency passes#83
shaug merged 1 commit into
mainfrom
scott/53-correctness-traversal-verification

Conversation

@shaug

@shaug shaug commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Fixes #53

Summary

  • Bump the shared review result contract to schema 1.3 (additive only): add
    a verification_sufficiency_evidence array recording, per claimed
    validation command or test touching a materially risky change, the
    claimed_test_or_command, whether it exercises_material_risk (yes,
    no, or not_applicable), and reasoning.
  • review-suite/scripts/validate.py: enforce structure and a clean-verdict
    consistency rule — an entry recording exercises_material_risk: "no"
    cannot pair with a clean verdict, since that would silently hide the exact
    gap this evidence exists to surface. Extend the stale-schema-version
    rejection so a v1.2 result now fails with a useful migration error.
  • skills/review-correctness/SKILL.md: add the two required passes from
    Add correctness traversal and verification-sufficiency passes #53's body as dimensions of this single lens — a consumer/impact-traversal
    pass that populates Add consumer/impact-traversal evidence to the shared review contract #52's consumer_impact_evidence, and a
    verification-sufficiency pass that populates the new evidence array — and
    explicitly rule out routing to any specialist module.
  • Add a new canonical fixture, verification-sufficiency-guard, modeled on
    the frozen baseline's verification-sufficiency miss (a release guard whose
    only claimed test exercises an already-safe branch instead of the actual
    owner-absent triggering condition).
  • Add two real-runtime forward-evaluation scenarios under
    skills/review-correctness/evals/ modeled on both frozen v1 baseline
    misses, plus fixture-conformance and no-specialist-module tests.
  • Migrate every canonical fixture, eval helper, and each bundled skill's
    standalone eval fixtures to schema 1.3; refresh the four bundled
    review-suite copies via just sync-contracts.

Why

Acceptance criteria (from #53's body)

  • A multi-file contract fixture is not clean when the changed producer is
    correct but an unchanged consumer remains incompatible. — covered by
    Add consumer/impact-traversal evidence to the shared review contract #52's existing consumer-impact-traversal fixture/tests plus new
    real-runtime eval standalone-sibling-call-site-traversal.
  • A verification-gap fixture is not clean when passing tests mock, stub,
    or bypass the material behavior the change addresses. — new
    verification-sufficiency-guard fixture plus real-runtime eval
    standalone-verification-sufficiency-gap.
  • No specialist module (security, concurrency-as-a-context, migration,
    operational, or UI) exists anywhere in review-correctness after this
    change. — verified by test_no_specialist_module_exists and manual
    repository search.
  • Every clean result contains completed consumer_impact_evidence and
    verification_sufficiency_evidence where applicable. — SKILL.md now
    requires both; demonstrated by real-runtime replay below.
  • Clean controls and rejected connector findings remain clean. — existing
    fixtures migrated and still pass; a fresh agent given only
    repository-convention-clean's packet stayed clean with no spurious
    findings under both new passes.
  • Deterministic tests and repeated real-runtime replay demonstrate the
    behavior on both target cases. — see Verification below.
  • The preregistered v2 scoring gate is understood as informational
    context for a later, separate re-scoring ticket, not a gate on this
    merge (per the ticket body's own framing). No scoring harness was built.
  • just format, just lint, and just test pass.

Non-goals honored

Verification

  • just format, just lint, and just test all pass (241 tests in
    review-suite/scripts/tests, unchanged in count from the prior commit; 8
    tests in skills/review-correctness/scripts/tests, up from 3).
  • just sync-contracts leaves all four bundled review-suite copies
    byte-identical to the canonical source.
  • python3 review-suite/scripts/evals/audit_corpus.py passes for all 7
    corpora (no case-identifier leakage into reviewer-visible contract text).
  • Real-runtime replay: a fresh, isolated agent given only the bundled
    skill/contract text and each eval's raw evidence (no access to the
    expected-result answer key) correctly reproduced the required verdict on
    both new standalone evals, twice each, and left the existing
    repository-convention-clean fixture clean under both new passes with no
    spurious findings.
  • No file under review-suite/evals/baseline/v1/ or review-suite/evals/v2/
    was touched; Make clean verdicts require passing validation and current-head lens evidence #51's lens_executions and Add consumer/impact-traversal evidence to the shared review contract #52's consumer_impact_evidence
    behavior are unchanged and covered by their existing tests, which still
    pass.
  • Initial review-code-change pass (fresh, read-only, raw-evidence-only
    context) returned a clean aggregate across all three lenses
    (solution-simplicity, correctness, code-simplicity) bound to this exact
    head/base.

🤖 Generated with Claude Code

## Summary
- Bump the shared review result contract to schema `1.3` (additive only): add
  a `verification_sufficiency_evidence` array recording, per claimed
  validation command or test touching a materially risky change, the
  `claimed_test_or_command`, whether it `exercises_material_risk` (`yes`,
  `no`, or `not_applicable`), and `reasoning`.
- `review-suite/scripts/validate.py`: enforce structure and a clean-verdict
  consistency rule — unlike `consumer_impact_evidence`, an entry recording
  `exercises_material_risk: "no"` cannot pair with a `clean` verdict, since
  that would silently hide the exact gap this evidence exists to surface.
  Extend the stale-schema-version rejection so a v1.2 result now fails with a
  useful migration error, mirroring the existing v1.0/v1.1 rejections.
- `skills/review-correctness/SKILL.md`: add the two required passes from
  #53's body as dimensions of this single lens — a consumer/impact-traversal
  pass that populates #52's `consumer_impact_evidence`, and a
  verification-sufficiency pass that populates the new evidence array — and
  explicitly rule out routing to any specialist module.
- Add a new canonical fixture, `verification-sufficiency-guard`, modeled on
  the baseline's `stale-claim-release-guard` miss (a release guard whose only
  claimed test exercises an already-safe branch instead of the actual
  owner-absent triggering condition), covering the required
  fixture/consistency scenarios in `review-suite/scripts/tests/test_contracts.py`.
- Add two real-runtime forward-evaluation scenarios under
  `skills/review-correctness/evals/` (`standalone-sibling-call-site-traversal`
  and `standalone-verification-sufficiency-gap`), modeled on both frozen v1
  baseline misses, plus fixture-conformance and no-specialist-module tests in
  `skills/review-correctness/scripts/tests/test_skill_contract.py`.
- Migrate every canonical fixture, eval helper, and each bundled skill's
  standalone eval fixtures to schema 1.3; refresh the four bundled
  `review-suite` copies via `just sync-contracts`.

## Why
- #59's decision record found both baseline misses in
  `s1-correctness-orchestrator` confident (zero ambiguity, 5-for-5 across
  independent attempts): a missed sibling call site of a changed
  permissive/strict helper, and a verification-sufficiency gap where the
  added test exercised an already-safe branch. #52 supplied the
  `consumer_impact_evidence` schema shape; this ticket supplies the lens
  behavior that populates and reasons from it, plus the new
  verification-sufficiency evidence array and pass. Specialist routing is
  dropped entirely per #59's decision record — no baseline evidence supports
  it, and reintroducing it later requires its own decision-record entry.

## Verification
- `just format`, `just lint`, and `just test` all pass (241 tests in
  `review-suite/scripts/tests`, unchanged in count from the prior commit;
  8 tests in `skills/review-correctness/scripts/tests`, up from 3).
- `just sync-contracts` leaves all four bundled `review-suite` copies
  byte-identical to the canonical source.
- `python3 review-suite/scripts/evals/audit_corpus.py` passes for all 7
  corpora (no case-identifier leakage into reviewer-visible contract text).
- Real-runtime replay: a fresh, isolated agent given only the bundled
  skill/contract text and each eval's raw evidence (no access to the
  expected-result answer key) correctly reproduced the required verdict on
  both new standalone evals, twice each, and left an existing clean fixture
  (`repository-convention-clean`) clean under both new passes with no
  spurious findings.
- No file under `review-suite/evals/baseline/v1/` or `review-suite/evals/v2/`
  was touched; #51's `lens_executions` and #52's `consumer_impact_evidence`
  behavior are unchanged and covered by their existing tests, which still
  pass. No specialist module (security, concurrency-as-a-context,
  compatibility/migration, operations, or UI) exists anywhere in
  `review-correctness`.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@shaug
shaug merged commit 85ccf13 into main Jul 29, 2026
1 check passed
@shaug
shaug deleted the scott/53-correctness-traversal-verification branch July 29, 2026 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add correctness traversal and verification-sufficiency passes

1 participant